Skip to content

Features to support surface physics#135

Merged
FlorianDeconinck merged 36 commits into
NOAA-GFDL:developfrom
oelbert:feature/lsm
May 13, 2025
Merged

Features to support surface physics#135
FlorianDeconinck merged 36 commits into
NOAA-GFDL:developfrom
oelbert:feature/lsm

Conversation

@oelbert
Copy link
Copy Markdown
Collaborator

@oelbert oelbert commented Apr 25, 2025

Description
This PR adds some features used in the SHiELD physics for surface routines, mostly namelist parameters, along with a new basic stencil for averaging fields and a tridiagonal solver.

How Has This Been Tested?
Surface physics schemes have been tested and validated with this branch

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • New check tests, if applicable, are included

oelbert and others added 30 commits June 11, 2024 11:26
@oelbert
new constants
45f6645
@oelbert
adding some stencils and the stefan-boltzman constant
453d033
@oelbert
adding namelist switches
5c1028b
@oelbert
merge develop
06ccaf4
@oelbert
Merge branch 'feature/4d_data' into feature/lsm
08047cd
@oelbert
add dtypes
d2bc643
@oelbert
update dtypes
0abfe57
@oelbert
hydlb
ee07993
@oelbert
add triple point temp
d5d65c1
@oelbert
Merge branch 'develop' into feature/lsm
171d123
@oelbert
cleanup of some constants
ac211bf
@oelbert
adding lsoil for lsm
ab690a6
@oelbert
add method to create array from compute domain
71b070c
@oelbert
try ndim not 4d
8294a88
@oelbert
iop
7bb467f
@oelbert
adding masked tridiag
cdb4588
@oelbert
added default stzrt to nd storage data
638ecdc
@oelbert
bool data handling
b1e249b
@oelbert
Merge branch 'feature/lsm' of github.com:oelbert/NDSL into feature/lsm
b33061e
@oelbert
merged develop
eb1b3de
@oelbert
oops
a7163db
@oelbert
added keyword to translate indices between Fortran and Python for tra…
4190029 
…nslate tests
@oelbert
ndsl/stencils/testing/translate.py
0e111a0
@oelbert
remove break
fb7fab7
@oelbert
adding triple point constant
9f2371b
@oelbert
merge
c9cf275
@oelbert
update namelist
d5d5021
@oelbert
oops
920370e
@oelbert
better default
0abee5d
@oelbert
Merge branch 'feature/index_tests' of github.com:oelbert/NDSL into fe…
d395d9d 
…ature/index_tests
romanc
romanc reviewed Apr 25, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@romanc romanc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linter seems unhappy 😢 Since I don't understand much of nameslists/constants and how they are used, I focused on nitpicking ;)

Would it make sense to have ndsl/stencils/solvers.py such that users can

from ndls.stencils.solvers import masked_tridiagonal_solver

write imports like the above one? If (in the future) we get to many, many solvers in the future, we could even do subfolders like

from ndsl.stencils.solvers.tridiagonal import thomas_algorithm

I'm just thinking out loud here. It might be too early to do that kind of name changes ...

I also think it would be nice to have a simple test for the tridiagonal solvers.

Comment thread ndsl/constants.py Outdated
Comment thread ndsl/stencils/basic_operations.py Outdated
Comment thread ndsl/stencils/tridiag.py Outdated
Comment thread ndsl/stencils/tridiag.py Outdated
Comment thread ndsl/stencils/tridiag.py Outdated
b: diagonal matrix coefficients
c: upper-diagonal matrix coefficients
d: Result vector
mask:
Copy link
Copy Markdown
Collaborator

@romanc romanc Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

description for mask is missing ...

Comment thread ndsl/stencils/tridiag.py
@oelbert
Copy link
Copy Markdown
Collaborator Author

oelbert commented May 9, 2025

File "/__w/NDSL/NDSL/pace/pace/state.py", line 14, in <module> from pyFV3 import DycoreState ImportError: cannot import name 'DycoreState' from 'pyFV3' (unknown location)

what?

@fmalatino
Copy link
Copy Markdown
Contributor

fmalatino commented May 9, 2025

File "/__w/NDSL/NDSL/pace/pace/state.py", line 14, in <module> from pyFV3 import DycoreState ImportError: cannot import name 'DycoreState' from 'pyFV3' (unknown location)

what?

@oelbert This error also popped up previously, update your base branch and you should be good, i think.

@romanc
Copy link
Copy Markdown
Collaborator

romanc commented May 10, 2025

File "/__w/NDSL/NDSL/pace/pace/state.py", line 14, in <module> from pyFV3 import DycoreState ImportError: cannot import name 'DycoreState' from 'pyFV3' (unknown location)

what?

NDSL has a hook to run tests from pace. These pace tests had issues with pip and editable installs (what Frank talked about in the last pace meeting). These issues were preliminarily resolved with NOAA-GFDL/pace#117. Your changes have nothing to do with this and just open a PR at the wrong moment. The test hook just pulls pace's latest state and with 117 merged the workflow runs again as it should. Updating the base branch of NDSL is the easiest way to re-trigger a pipeline run and this is why it seems to fix the problem. But actually, all of this has nothing to do with any (of your) changes in NDSL.

FlorianDeconinck
FlorianDeconinck approved these changes May 13, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@FlorianDeconinck FlorianDeconinck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@FlorianDeconinck FlorianDeconinck merged commit 4cda272 into NOAA-GFDL:develop May 13, 2025
5 checks passed
@oelbert oelbert deleted the feature/lsm branch August 14, 2025 18:50
@oelbert oelbert restored the feature/lsm branch August 14, 2025 18:51
@oelbert oelbert deleted the feature/lsm branch August 14, 2025 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@romanc romanc romanc left review comments

@FlorianDeconinck FlorianDeconinck FlorianDeconinck approved these changes

@fmalatino fmalatino Awaiting requested review from fmalatino

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@oelbert @fmalatino @romanc @FlorianDeconinck